Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Simplified dictionary indexes #584

Merged
merged 1 commit into from
Nov 7, 2021
Merged

Simplified dictionary indexes #584

merged 1 commit into from
Nov 7, 2021

Conversation

jorgecarleitao
Copy link
Owner

The Box<DataType> in DataType::Dictionary(Box<DataType>, _) is not sufficiently restrictive as there are only 8 possible variants on the dictionary keys. This PR changes it to DataType::Dictionary(IntegerType, _), allowing match to cover all cases.

Backward incompatible changes

As a user declaring dictionaries, to migrate to the new behavior, change

DataType::Dictionary(Box::new(DataType::Int32), Box::new(values))

to

DataType::Dictionary(i32::KEY_TYPE, Box::new(values))

As a user writing kernels, use arrow2::datatypes::IntegerType to match the cases.

@codecov
Copy link

codecov bot commented Nov 7, 2021 •
edited
Loading

Codecov Report

Merging #584 (7cc30b6) into main (f146097) will increase coverage by 0.05%.
The diff coverage is 56.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
+ Coverage   78.99%   79.05%   +0.05%     
==========================================
  Files         399      399              
  Lines       24752    24736      -16     
==========================================
+ Hits        19554    19555       +1     
+ Misses       5198     5181      -17
Impacted Files Coverage Δ
src/array/growable/mod.rs 21.56% <0.00%> (-4.63%) ⬇️
src/compute/aggregate/memory.rs 20.51% <0.00%> (ø)
src/compute/cast/dictionary_to.rs 24.39% <0.00%> (-0.61%) ⬇️
src/io/avro/read/nested.rs 44.44% <ø> (ø)
src/io/json_integration/mod.rs 0.00% <ø> (ø)
src/types/mod.rs 23.61% <ø> (ø)
src/array/ord.rs 71.25% <22.22%> (ø)
src/array/display.rs 50.37% <25.00%> (+<0.01%) ⬆️
src/array/mod.rs 59.64% <25.00%> (ø)
src/io/csv/write/serialize.rs 33.93% <33.33%> (ø)
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f146097...7cc30b6. Read the comment docs.

@jorgecarleitao jorgecarleitao changed the title Simplified dictionary indexes. Simplified dictionary indexes Nov 7, 2021
@jorgecarleitao jorgecarleitao marked this pull request as ready for review November 7, 2021 17:53
@jorgecarleitao jorgecarleitao merged commit 9041c04 into main Nov 7, 2021
@jorgecarleitao jorgecarleitao deleted the index branch November 7, 2021 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
backwards-incompatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jorgecarleitao